fix(kafka): fix use-after-free on task cancellation during kafka producer Send#1299
fix(kafka): fix use-after-free on task cancellation during kafka producer Send#1299disaykin wants to merge 2 commits into
Conversation
…ucer Send Fixes a Use-After-Free vulnerability in the asynchronous Kafka Producer API when a task is cancelled before librdkafka flushes its queue. The `kafka::Producer::Send` API accepts parameters as views (`string_view`, `zstring_view`, `HeaderViews`). These views point to data owned by the caller. Inside `Send`, raw pointers from these views are passed down to `librdkafka`, which stores them in its internal asynchronous queues for background delivery. If a client connection drops, a cascade cancellation of tasks occurs. This leads to a premature return from `kafka::Producer::Send`, causing the caller to destroy the original payload memory. However, `librdkafka` still retains these pointers and attempts to access them during subsequent network flushes to the broker, resulting in memory corruption. To fix this, we ensure that `Producer::Send` is blocked from returning prematurely and cannot exit until the background `SendImpl` completes its interaction with `librdkafka`. Co-authored-by: Aleksander Rypalov <rypalov2002@gmail.com>
30e88de to
82e223b
Compare
Old URL returns 404 now
|
@apolukhin прошу ревью. проблему с установкой кафки в тестах я починил (там протух урл), но вот что делать с флапающим тестом на постгрю - не знаю: Но это точно не относится к моему фиксу. Также проблема компиляции с rabbitmq на debian12 привнесена не мной/ |
|
Tests in postgres is flap. |
| set -o errexit -o nounset -o pipefail -o posix -x | ||
|
|
||
| KAFKA_VERSION=4.0.1 | ||
| KAFKA_VERSION=4.3.1 |
There was a problem hiding this comment.
Why did you increas it?
There was a problem hiding this comment.
Старая ссылка для установки кафки больше не работает. Заменил на ссылку для скачивания с их официального сайта
There was a problem hiding this comment.
Предлагаете оформить это отдельным запросом на влитие? Или претензия именно к изменению версии? Старая версия у меня вроде бы не захотела скачиваться по новой ссылке...
There was a problem hiding this comment.
Если у них на сайте теперь написано, что надо так, то окей
| set -o errexit -o nounset -o pipefail -o posix -x | ||
|
|
||
| KAFKA_VERSION=4.0.1 | ||
| KAFKA_VERSION=4.3.1 |
There was a problem hiding this comment.
Если у них на сайте теперь написано, что надо так, то окей
| ) const { | ||
| engine::TaskCancellationBlocker blocker; | ||
|
|
||
| if (engine::current_task::IsCancelRequested()) |
There was a problem hiding this comment.
Добавь тест, в котором таска отменяется и происходит use-after-free. Чтобы хотя бы локально убедиться, что бага есть, а после фикса её нет
There was a problem hiding this comment.
Это будет флаки тест. Даже при наличии бага, он будет фейлиться не каждый раз. И фэйлиться будет только под адрес-санитайзером. Я пробовал написать, получается как-то не очень... Попробую откопать свои наработки, может очередная попытка родит что-то удачнее
There was a problem hiding this comment.
О, придумал. Можно выставить большие значения либрдкафка накопления очереди перед сбросом (1000 сообщений, 1 секунда, что раньше наступит). Тогда тест будет не флаки под санитайзером
There was a problem hiding this comment.
Вообще,я имел ввиду, чтобы ты показал нам тест и локально убедился, что он падает без твоего фикса. А с фиксом чинится (и потом его удалить и не мержить). Но если получится даже полноцнный сделать, то шикарно
2 participants
Fixes a Use-After-Free vulnerability in the asynchronous Kafka Producer API when a task is cancelled before librdkafka flushes its queue.
The
kafka::Producer::SendAPI accepts parameters as views (string_view,zstring_view,HeaderViews). These views point to data owned by the caller. InsideSend, raw pointers from these views are passed down tolibrdkafka, which stores them in its internal asynchronous queues for background delivery.If a client connection drops, a cascade cancellation of tasks occurs. This leads to a premature return from
kafka::Producer::Send, causing the caller to destroy the original payload memory. However,librdkafkastill retains these pointers and attempts to access them during subsequent network flushes to the broker, resulting in memory corruption.To fix this, we ensure that
Producer::Sendis blocked from returning prematurely and cannot exit until the backgroundSendImplcompletes its interaction withlibrdkafka.Note: by creating a PR or an issue you automatically agree to the CLA. See CONTRIBUTING.md. Feel free to remove this note, the agreement holds.